Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwrap script validity field in TxBodyContent #3091

Merged
merged 1 commit into from
Aug 22, 2021

Conversation

newhoggy
Copy link
Contributor

No description provided.

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Maybe let Duncan have a look as well.

Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good so far, but we have to plumb the flag through to the right place so we can properly have the flag value go back and forth with the underlying ledger tx type (which does contain the flag).

type instance Core.Tx (AlonzoEra c) = ValidatedTx (AlonzoEra c)

data ValidatedTx era = ValidatedTx
  { body :: !(Core.TxBody era),
    wits :: !(TxWitness era),
    isValidating :: !IsValidating,
    auxiliaryData :: !(StrictMaybe (Core.AuxiliaryData era))
  }

So as you see, the flag value is directly available.

Comment on lines 1596 to 1602
pattern TxBody txbodycontent <- (getTxBodyContent TxScriptValidityNone -> txbodycontent)
{-# COMPLETE TxBody #-}

getTxBodyContent :: TxBody era -> TxBodyContent ViewTx era
getTxBodyContent (ByronTxBody body) = getByronTxBodyContent body
getTxBodyContent (ShelleyTxBody era body _scripts _redeemers mAux _scriptValidity) =
fromLedgerTxBody era body mAux
getTxBodyContent :: TxScriptValidity era -> TxBody era -> TxBodyContent ViewTx era
getTxBodyContent _ (ByronTxBody body) = getByronTxBodyContent body
getTxBodyContent scriptValidity (ShelleyTxBody era body _scripts _redeemers mAux _scriptValidity) =
fromLedgerTxBody era scriptValidity body mAux
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we would have to add the validity flag into the TxBody type, probably as an extra field in the ShelleyTxBody constructor. We have to get the value from the underlying ledger tx type, and not invent it. For one thing we need to do that to have the TxBody round trip test pass, but more importantly we need it to get the right value for the tx from a block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also getAlonzoTxBody where we extract the parts of the Alonzo.ValidatedTx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remove the argument of getTxBodyContent and am taking it from TxBody.

@newhoggy newhoggy force-pushed the unwrap-script-validity-in-tx-body-content branch 2 times, most recently from 231f435 to 95e5452 Compare August 19, 2021 11:53
@newhoggy newhoggy requested a review from dcoutts August 19, 2021 11:55
@newhoggy newhoggy dismissed dcoutts’s stale review August 19, 2021 12:11

Comments addressed

@newhoggy
Copy link
Contributor Author

bors merge

iohk-bors bot added a commit that referenced this pull request Aug 20, 2021
3091: Unwrap script validity field in TxBodyContent r=newhoggy a=newhoggy



Co-authored-by: John Ky <john.ky@iohk.io>
@newhoggy newhoggy force-pushed the unwrap-script-validity-in-tx-body-content branch from 95e5452 to 528a573 Compare August 20, 2021 00:54
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 20, 2021

Canceled.

@newhoggy newhoggy force-pushed the unwrap-script-validity-in-tx-body-content branch 2 times, most recently from e5efa5a to 31e6c75 Compare August 20, 2021 04:37
Copy link
Contributor

@MarcFontaine MarcFontaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@newhoggy newhoggy force-pushed the unwrap-script-validity-in-tx-body-content branch from 31e6c75 to 80cf2f1 Compare August 21, 2021 05:16
@newhoggy
Copy link
Contributor Author

bors merge

iohk-bors bot added a commit that referenced this pull request Aug 21, 2021
3091: Unwrap script validity field in TxBodyContent r=newhoggy a=newhoggy



Co-authored-by: John Ky <john.ky@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 21, 2021

Build failed:

@newhoggy newhoggy force-pushed the unwrap-script-validity-in-tx-body-content branch from 80cf2f1 to 45ce7fd Compare August 22, 2021 07:13
@newhoggy
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 22, 2021
3091: Unwrap script validity field in TxBodyContent r=newhoggy a=newhoggy



Co-authored-by: John Ky <john.ky@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 22, 2021

Build failed:

@johnalotoski
Copy link
Contributor

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 22, 2021

@iohk-bors iohk-bors bot merged commit a52f20e into master Aug 22, 2021
@iohk-bors iohk-bors bot deleted the unwrap-script-validity-in-tx-body-content branch August 22, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants